Skip to content

fix: add SHA-256 checksum verification to GitHub release downloads#288

Open
xiaolai wants to merge 1 commit into
nextlevelbuilder:mainfrom
xiaolai:fix/nlpm-github-checksum-verify
Open

fix: add SHA-256 checksum verification to GitHub release downloads#288
xiaolai wants to merge 1 commit into
nextlevelbuilder:mainfrom
xiaolai:fix/nlpm-github-checksum-verify

Conversation

@xiaolai

@xiaolai xiaolai commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Automated audit: This PR was generated by NLPM, a natural language programming linter, running via claude-code-action. Please evaluate the diff on its merits.

Security Fix (Medium)

cli/src/utils/github.tsdownloadRelease() fetches a ZIP from GitHub releases and writes it directly to disk without any integrity check. init.ts then extracts and installs the contents into the user's project. A compromised release asset (e.g. from a hijacked GitHub token or a compromised release pipeline) would be silently installed with no way for the user to detect the tamper.

Fix: This PR adds opt-in SHA-256 verification using a companion checksum asset pattern:

  1. getChecksumUrl(release, assetName) — looks for a <asset-name>.sha256 or checksums.txt asset in the GitHub release
  2. downloadRelease(url, dest, checksumUrl?) — if checksumUrl is provided and the fetch succeeds, computes SHA-256 of the downloaded buffer and compares against the expected hash before writing to disk; throws GitHubChecksumError on mismatch
  3. init.ts now calls getChecksumUrl(release, assetName) and passes the result to downloadRelease

This is backward-compatible: if no companion checksum asset is uploaded, the behavior is identical to the current code. To enable full verification, upload a release.zip.sha256 (or checksums.txt) companion file to each GitHub release — the CLI will then automatically verify before installation.

downloadRelease() previously wrote the downloaded ZIP to disk without any
integrity check. A compromised release asset would be silently installed
into user projects.

Changes:
- Add getChecksumUrl() helper that looks for a companion `.sha256` or
  `checksums.txt` asset in the release
- Add optional checksumUrl parameter to downloadRelease(); if a checksum
  URL is provided and the fetch succeeds, compute SHA-256 of the downloaded
  buffer and compare before writing to disk
- Add GitHubChecksumError class for mismatch failures
- Update init.ts to look up and pass the checksum URL when present

This is forward-compatible: if no checksum asset is uploaded the behavior
is unchanged. Maintainers can enable full verification by uploading a
`<asset-name>.sha256` companion file to each release.

Co-Authored-By: Claude Code <noreply@anthropic.com>

@mrgoonie mrgoonie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary: Optional release checksum verification is a good security direction, but this PR duplicates an adjacent open checksum PR and the implementation has an unsafe fallback that silently ignores checksum fetch failures.

Risk level: Medium

Mandatory gates:

  • Duplicate/prior implementation: overlap found — PR #285 implements the same SHA-256 release download verification path against the same files, and PR #315 also includes broader release/download hardening.
  • Project standards: issue found — the CLI currently treats GitHub download failures as install blockers or explicit fallback paths; checksum verification should not look enabled while silently skipping a failed checksum fetch.
  • Strategic necessity: clear value — verified release assets would reduce supply-chain risk for the legacy GitHub install path.
  • CI/checks: missing/not reported for this PR.

Findings:

  • Important: This materially duplicates PR #285. Both PRs modify `cli/src/commands/init.ts` and `cli/src/utils/github.ts` to add optional SHA-256 verification for release downloads. Please consolidate around one checksum design instead of merging competing implementations.
  • Important: When a checksum asset exists but fetching it fails, `downloadRelease()` currently proceeds without verification because it only checks `if (checksumResponse.ok)` and otherwise falls through to `writeFile()`. That creates a dangerous false sense of integrity protection: a transient or blocked checksum fetch silently downgrades security. If a checksum URL was selected, failure to fetch or parse it should fail closed with a clear checksum/download error.

Verdict: REQUEST_CHANGES

@mrgoonie mrgoonie added agent:github-maintain Processed by github-maintain automation pr:reviewed PR reviewed by maintain workflow pr:changes-requested Maintain review requested changes labels Jun 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:github-maintain Processed by github-maintain automation pr:changes-requested Maintain review requested changes pr:reviewed PR reviewed by maintain workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants